Skip to content

Add layered MergePipeline for multi-source entity discovery#246

Open
bburda wants to merge 11 commits intomainfrom
feature/merge-pipeline
Open

Add layered MergePipeline for multi-source entity discovery#246
bburda wants to merge 11 commits intomainfrom
feature/merge-pipeline

Conversation

@bburda
Copy link
Collaborator

@bburda bburda commented Mar 4, 2026

Pull Request

Summary

Replace ad-hoc merging in HybridDiscoveryStrategy with a formal layered MergePipeline that orchestrates manifest, runtime, and plugin discovery sources with configurable per-field-group merge precedence (AUTHORITATIVE/ENRICHMENT/FALLBACK). Adds conflict detection, gap-fill filtering, runtime linker determinism fixes, and diagnostic reporting via /health endpoint.

Commit overview

  1. feat(discovery): add layered MergePipeline with field-group merge - MergePolicy/FieldGroup enums, MergeReport, DiscoveryLayer interface, MergePipeline core with single/multi-layer merge and conflict resolution
  2. feat(discovery): add ManifestLayer, RuntimeLayer, and PluginLayer - Concrete layer wrappers with per-field-group policies, GapFillConfig for namespace filtering
  3. feat(discovery): add post-merge RuntimeLinker and wire into HybridDiscoveryStrategy - App-to-node binding with multi-match detection, orphan warnings, deterministic namespace matching; replaces ad-hoc merge logic in HybridDiscoveryStrategy
  4. feat(discovery): add plugin integration, config, and /health reporting - IntrospectionProvider integration, YAML gap-fill config, MergeReport in /health, IntrospectionInput context passing, comprehensive docs
  5. test(plugins): add vendor extension demo and integration test - VendorExtensionPlugin demo + integration test for plugin layer discovery
  6. fix(build): enable POSITION_INDEPENDENT_CODE for MODULE targets - Fix TPOFF32/R_X86_64_PC32 linker errors for test_gateway_plugin.so

Issue


Type

  • Bug fix
  • New feature or tests
  • Breaking change
  • Documentation only

Testing

  • 80 unit tests covering merge pipeline (42) and runtime linker (38)
  • All 2410 tests passing across 7 packages (0 failures)
  • 30 integration tests passing
  • Tests cover: single/multi/three-layer merge, all merge policies (AUTH/ENRICH/FALLBACK), conflict detection, gap-fill namespace filtering (whitelist/blacklist with path-segment boundaries), App STATUS bool-or semantics, layer exception safety, orphan policies (ERROR/WARN/IGNORE/INCLUDE_AS_ORPHAN), wildcard determinism, node exclusivity, binding conflicts, plugin context passing

Checklist

  • Breaking changes are clearly described (and announced in docs / changelog if needed)
  • Tests were added or updated if needed
  • Docs were updated if behavior or public API changed

Copilot AI review requested due to automatic review settings March 4, 2026 19:26
@bburda bburda marked this pull request as draft March 4, 2026 19:27
@bburda bburda self-assigned this Mar 4, 2026
@bburda bburda added enhancement New feature or request discovery Discovery endpoints or strategies labels Mar 4, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces a formal, layered discovery merge pipeline for HYBRID discovery mode, replacing the prior ad-hoc hybrid merging/linking approach and exposing pipeline diagnostics via the /health endpoint.

Changes:

  • Added DiscoveryLayer + concrete layers (ManifestLayer, RuntimeLayer, PluginLayer) and a MergePipeline with per-field-group merge policies, conflict/collision detection, and gap-fill filtering.
  • Updated HYBRID discovery to cache merged results and to run a post-merge RuntimeLinker step with deterministic namespace/topic matching and additional conflict reporting.
  • Exposed merge-pipeline diagnostics in /api/v1/health and added extensive unit tests for the pipeline and linker behavior.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/ros2_medkit_gateway/test/test_runtime_linker.cpp Adds deterministic namespace/topic matching and conflict scenario tests for RuntimeLinker.
src/ros2_medkit_gateway/test/test_merge_pipeline.cpp New unit tests covering merge policies, layers, gap-fill config, plugin layer behavior, and post-merge linking.
src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp Adds API to return introspection providers along with their plugin names.
src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp Adds discovery mode/strategy and merge-pipeline report summary to /health.
src/ros2_medkit_gateway/src/gateway_node.cpp Adds merge-pipeline gap-fill params; registers introspection plugins as pipeline layers in HYBRID; adjusts cache refresh flow.
src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp Implements merge pipeline execution, per-field-group merges, conflicts, ID collision logging, and post-merge app-to-node linking.
src/ros2_medkit_gateway/src/discovery/manifest/runtime_linker.cpp Adds path-segment-boundary matching, deterministic candidate selection, and binding conflict counters/warnings.
src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp New runtime discovery layer with gap-fill filtering and default merge policies.
src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp New plugin discovery layer wrapping IntrospectionProvider output.
src/ros2_medkit_gateway/src/discovery/layers/manifest_layer.cpp New manifest discovery layer wrapping ManifestManager.
src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp Refactors HYBRID discovery to use and cache MergePipeline output.
src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp Constructs pipeline in HYBRID mode, refreshes pipeline on topic-map refresh, supports adding plugin layers, and exposes merge reports.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp Declares the new named introspection provider API.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp Adds merge policy/types, merge report structure, and gap-fill config.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp Declares the MergePipeline interface and merge execution result.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp Extends linking result stats/summary for conflicts and wildcard multi-match.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp Declares the runtime discovery layer wrapper and gap-fill filtering support.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp Declares the plugin discovery layer wrapper for introspection providers.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/manifest_layer.hpp Declares the manifest discovery layer wrapper.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp Updates HYBRID strategy interface to pipeline-based caching and reporting.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_manager.hpp Adds merge-pipeline config, plugin-layer API, and merge-report accessors.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp Introduces the DiscoveryLayer interface and LayerOutput struct.
src/ros2_medkit_gateway/config/gateway_params.yaml Documents new merge-pipeline gap-fill configuration parameters.
src/ros2_medkit_gateway/CMakeLists.txt Adds new pipeline/layer sources and new test_merge_pipeline target.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.

@bburda bburda force-pushed the feature/merge-pipeline branch from 5484fe5 to 8b47dd7 Compare March 4, 2026 20:08
@bburda bburda marked this pull request as ready for review March 4, 2026 21:15
@bburda bburda requested review from Copilot and mfaferek93 March 4, 2026 21:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.

@bburda bburda requested a review from Copilot March 5, 2026 16:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

bburda added 6 commits March 5, 2026 20:52
Introduce MergePolicy (AUTHORITATIVE/ENRICHMENT/FALLBACK), FieldGroup enums,
MergeReport diagnostics, DiscoveryLayer interface and LayerOutput struct.
Implement MergePipeline core with single-layer passthrough and multi-layer
field-group merge with conflict resolution.
Implement concrete DiscoveryLayer wrappers: ManifestLayer (delegates to
ManifestManager, AUTHORITATIVE for identity/hierarchy/metadata), RuntimeLayer
(delegates to RuntimeDiscoveryStrategy, AUTHORITATIVE for live_data/status),
PluginLayer (wraps IntrospectionProvider, ENRICHMENT for all fields).
Add GapFillConfig for RuntimeLayer namespace filtering in hybrid mode.
…coveryStrategy

Implement RuntimeLinker that binds manifest apps (with ros_binding) to
runtime-discovered ROS 2 nodes (with bound_fqn) after pipeline merge.
Includes multi-match detection, binding conflict reporting, orphan node
warnings, and deterministic namespace matching. Wire MergePipeline into
HybridDiscoveryStrategy replacing the ad-hoc merge logic.
Integrate IntrospectionProviders as pipeline layers with priority ordering
and batch registration. Add YAML configuration for merge pipeline gap-fill.
Expose MergeReport and LinkingResult in GET /health endpoint. Pass merged
entity context (IntrospectionInput) to plugin layers before discover().
Add comprehensive documentation for merge pipeline architecture, policies,
gap-fill options, and merge report diagnostics.
Add VendorExtensionPlugin demo that registers custom entities and routes
via the IntrospectionProvider interface. Add integration test validating
plugin layer discovery, entity merging, and context passing through the
merge pipeline.
Fix linker errors (TPOFF32, R_X86_64_PC32) when gateway_lib.a is linked
into test_gateway_plugin.so (MODULE target). Remove static thread_local
std::mt19937 in BulkDataStore (incompatible with initial-exec TLS model).
Enable POSITION_INDEPENDENT_CODE on ros2_medkit_serialization static lib.
Remove @verifies tag referencing nonexistent REQ_DISC_MERGE_FUNCTION.
@bburda bburda force-pushed the feature/merge-pipeline branch from 61874c0 to e886342 Compare March 5, 2026 20:01
@bburda bburda changed the title feat(discovery): add layered MergePipeline for multi-source entity discovery Add layered MergePipeline for multi-source entity discovery Mar 5, 2026
bburda added 2 commits March 6, 2026 15:02
…components, docs

Fix per-field-group owner tracking in MergePipeline::merge_entities().
Previously, all field groups used the first layer as comparison target
for subsequent merges. Now each field group tracks its current owning
layer independently, preventing lower-priority layers from incorrectly
overriding authoritative values won by middle layers (e.g., Plugin
ENRICH overriding Runtime AUTH for STATUS).

Restore create_synthetic_components config check in discover_components()
so that setting it to false produces per-node components (legacy mode)
instead of unconditionally grouping by namespace.

Update manifest-schema.rst and manifest-discovery.rst to document
path-segment-boundary namespace matching semantics (was documented as
"exact match" but code uses prefix matching with segment boundaries).
…config

Add configurable per-layer policy overrides so users can customize merge
behavior for each field group (identity, hierarchy, live_data, status,
metadata) on manifest and runtime layers independently.

- Add field_group_from_string() and merge_policy_from_string() parsers
- Add layer_policies map to MergePipelineConfig
- Add apply_layer_policy_overrides() template in DiscoveryManager
- Wire parameter declaration and reading in GatewayNode
- Uncomment layers section in gateway_params.yaml with defaults
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.

- Fix YAML param naming mismatch: use dot-notation (discovery.mode,
  discovery.manifest_path) matching gateway_params.yaml structure
- Make hybrid mode fail on missing/invalid manifest (same as manifest_only)
- Add WARN logs for unknown discovery.mode enum values
- Fix stale param references in tutorials and config docs
- Remove stale expose_nodes_as_apps from discovery-options.rst
- Add unit tests for FieldGroup/MergePolicy parsing and layer policy overrides
- Add integration tests for gap-fill, namespace filter, layer policies,
  and legacy discovery mode
- Update existing integration tests with corrected param names
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 47 out of 47 changed files in this pull request and generated 10 comments.

@bburda bburda marked this pull request as draft March 6, 2026 15:01
Each integration test now gets a unique GATEWAY_TEST_PORT from CMake
(stride of 10, starting at 9100), eliminating port conflicts during
parallel CTest execution. All tests use get_test_port() consistently
instead of hardcoded ports.

Also fixes the 4 new discovery integration tests:
- Correct poll_endpoint_until lambda pattern (return data, not bool)
- Use collection response format (data['items'])
- Add explicit timeouts for discovery-dependent polls
- Fix server.port param name in create_gateway_node helper
@bburda bburda marked this pull request as ready for review March 6, 2026 15:36
@bburda bburda requested a review from Copilot March 6, 2026 15:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated 2 comments.

…spection, safety

- Fix data race in HybridDiscoveryStrategy: get_linking_result() now
  reads from cached_result_ (protected by mutex) instead of pipeline_
  internals that execute() mutates without synchronization
- Eliminate double discover_apps() in RuntimeLayer: apps are discovered
  once and passed to discover_components() via new overload
- Add provides_runtime_apps() virtual to DiscoveryLayer, replacing
  fragile name-based "runtime" check that a plugin could collide with
- Use .at() instead of operator[] in RuntimeLinker binding-conflict
  warning to prevent accidental map insertion
- Add missing #include <cctype> in plugin_layer.cpp for std::isalnum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discovery Discovery endpoints or strategies enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discovery: Implement merge pipeline for discovery hybrid approach

2 participants